Skip to content

[CW2-31] refactor code #21

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 13 commits into from
Jul 25, 2024
Merged

[CW2-31] refactor code #21

merged 13 commits into from
Jul 25, 2024

Conversation

derekxu04
Copy link
Contributor

@derekxu04 derekxu04 commented Jul 4, 2024 •
edited
Loading

Changes:

  • Fixed assets not showing up on some pages
  • Refactored data.ts format
  • Created components for Layout, PageBody and PageTitle
  • Some fixes to hamburger and pages
  • Changed about description to be more comprehensive
  • Changed breadcrumb -> this is probably the biggest change (I thought the previous breadcrumb didn't really make sense, only worry is this might be too repetitive given the logo above? not sure)

image

@derekxu04 derekxu04 requested a review from andrew-l3 July 22, 2024 08:52
@derekxu04 derekxu04 changed the title DRAFT: CW2-31 refactor code CW2-31 refactor code Jul 22, 2024
@derekxu04 derekxu04 changed the title CW2-31 refactor code [CW2-31] refactor code Jul 22, 2024
andrew-l3
andrew-l3 approved these changes Jul 24, 2024
View reviewed changes
Copy link
Contributor

@andrew-l3 andrew-l3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good. Going down the refactoring path, we could probably simplify components in files like execs-directors-subcoms.tsx and constitution.tsx (i.e. add a component and move its data to an existing .ts file like before) but that can be done down the line.

@derekxu04 derekxu04 merged commit a4b12d8 into master Jul 25, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrew-l3 andrew-l3 andrew-l3 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@derekxu04 @andrew-l3